-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix(radio): remove getNativeControl from adapter #3785
fix(radio): remove getNativeControl from adapter #3785
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3785 +/- ##
==========================================
- Coverage 98.52% 98.52% -0.01%
==========================================
Files 120 120
Lines 5236 5224 -12
Branches 658 657 -1
==========================================
- Hits 5159 5147 -12
Misses 77 77
Continue to review full report at Codecov.
|
All 557 screenshot tests passed for commit 1b0feca vs. |
All 560 screenshot tests passed for commit 92cb7c7 vs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few nits, otherwise LGTM!
packages/mdc-radio/README.md
Outdated
| `removeClass(className: string) => void` | Removes a class from the root element | | ||
Method Signature | Description | ||
--- | --- | ||
`setNativeControlDisabled() => void` | Sets the input to disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing disabled: boolean
argument.
Also update description to something like:
Sets the input's `disabled` property to the given value
assert.isTrue(radio.disabled); | ||
}); | ||
|
||
test('#adapter.setNativeControlDisabled returns false when checkbox is not disabled', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the wording of this description a little confusing.
Maybe rephrase it to:
"#adapter.setNativeControlDisabled sets the native control element's disabled property to false"
assert.isFalse(root.classList.contains('foo')); | ||
}); | ||
|
||
test('#adapter.setNativeControlDisabled sets the native control element disabled', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the wording of this description a little confusing.
Maybe rephrase it to:
"#adapter.setNativeControlDisabled sets the native control element's disabled property to true"
All 560 screenshot tests passed for commit bd18aef vs. |
BREAKING CHANGE: Removed getNativeControl from adapter, and subsequent foundation methods that called getNativeControl. Foundation methods removed: isChecked, setChecked, isDisabled, getValue, setValue.
fixes #3763
related #2684
BREAKING CHANGE: Removed
getNativeControl
from adapter, and subsequent foundation methods that calledgetNativeControl
. Foundation methods removed:isChecked
,setChecked
,isDisabled
,getValue
,setValue
.